Conversation
src/lib.rs
Outdated
|
|
||
| /// Shorthand method to convert an argument into a [Box<serde_json::value::RawValue>]. | ||
| /// Since serializers rarely fail, it's probably easier to use [arg] instead. | ||
| /// Shorthand method to convert an argument into a [`Box<serde_json::value::RawValue>`]. |
There was a problem hiding this comment.
Does this work? I thought links can only point to identifiers. Taking user to Box is not very useful here, I think.
There was a problem hiding this comment.
Woops, changed to "... a boxed [serde_json::value::RawValue]" (but with backticks)
| /// Since serializers rarely fail, it's probably easier to use [arg] instead. | ||
| /// Shorthand method to convert an argument into a [`Box<serde_json::value::RawValue>`]. | ||
| /// | ||
| /// Since serializers rarely fail, it's probably easier to use [`arg`] instead. |
There was a problem hiding this comment.
Just arg, no? It's not a identifier so it can't be a link.
There was a problem hiding this comment.
This one is correct, arg is a function as well. This threw me too, perhaps we should change the paramater identifier to not be arg?
| //! This module implements a minimal and non standard conforming HTTP 1.0 | ||
| //! round-tripper that works with the bitcoind RPC server. This can be used | ||
| //! if minimal dependencies are a goal and synchronous communication is ok. | ||
| //! |
There was a problem hiding this comment.
I did this so every file has the same comment layout. This is copied from rust-bitcoin where I also did it to make everything uniform - pretty anal I know. I did these docs fixes in preparation for bring this repo into the rust-bitcoin org.
|
|
||
| impl SimpleHttpTransport { | ||
| /// Construct a new `SimpleHttpTransport` with default parameters | ||
| /// Constructs a new [`SimpleHttpTransport`] with default parameters. |
There was a problem hiding this comment.
Imperative mood is idiomatic, AFAIK.
There was a problem hiding this comment.
I certainly hope not, I've been changing everything to third person for the last 10 months in rust-bitcoin :)
There was a problem hiding this comment.
Oh my...
... I just checked stdlib ... and indeed ...
... I've been living a lie.
src/lib.rs
Outdated
| } | ||
|
|
||
| /// Shorthand method to convert an argument into a [Box<serde_json::value::RawValue>]. | ||
| /// Shorthand method to convert an argument into a [`Box<serde_json::value::RawValue>`]. |
|
Thanks for the review @dpc! Hope my responses weren't too terse, I'm in the last few minutes of my day. |
2bda9e4 to
c041572
Compare
|
Force push with no changes (change to commit hash only) to trigger CI. |
|
Will need #74 for the clippy CI fails, for the MacOS fails I don't know whats wrong. |
Ha! We use this already in this repo but by pure coincidence I stumbled across broken links in |
|
Needs rebase to resolve conflicts and to fix CI. |
Audit the whole codebase and improved rustdocs by doing: - Use third person tense - Use full stops - Use backticks on links - Use standard rustdoc section headings
Linking to `Box` in the rustdocs is pointless, link to the inner `serder_json` type instead.
c041572 to
ab44396
Compare
|
Cheers, rebased. No other changes. |
Audit the whole codebase and improved rustdocs by doing: